@psavery Marking as WIP for now, I'm getting bugs in testing. I'll tag you again once they're fixed.

@psavery @saransh13 Using NaNs in the raw view requires that all raw image data be floats. Will there be issues if we cast data to float?

@psavery This "fixes" the issue... although I'm not sure if it is the solution we want. Right now, before masking, we check to see if the image data type in integer and we cast it to float if it is. Should this maybe just automatically happen on image load so we only store the raw image data as floats?

@bnmajor I'll check on this in more detail soon. Maybe one solution would be to used a masked array instead, and different parts of the code can treat the mask differently (like the raw image viewer can convert the masked array to a plain array with nans).

Or maybe matplotlib accepts masked arrays, and we can just set the fill value to nan and give it to matplotlib and let matplotlib take care of it.

So here are the places where that HexrdConfig().masked_images_dict is being used. We would need to confirm that all of them work properly if we modify HexrdConfig().masked_images_dict:

• The fast powder calibration
• Laue and powder calibration (used during auto-picking for both powder and Laue, and used for the actual calibration)
• Stereo view drawing from raw (although that is currently not working anyways)
• The raw view

If we modify the output of masked_images_dict, we need to verify that all of these work properly.

Alternatively, I think we can do something else that will require less checking. Can we take the logic in masked_images_dict, and put it inside a new function on HexrdConfig() called create_masked_images_dict? That create_masked_images_dict will have a single argument fill_value that defaults to 0. HexrdConfig().masked_images_dict will return the output of that function with the default argument.

Then, in the image canvas code here, we can return HexrdConfig().create_masked_images_dict(fill_value=np.nan). Then in create_masked_images_dict(), we can have that logic to convert to a float if the fill value is a float (including np.nan, but only if there are masks present).

That's just a possibility for a less-invasive change. We can still modify the output of HexrdConfig().masked_images_dict, but if we do, we need to verify that everything is working properly.

By the way, about the masked arrays, it looks like you can't set a fill value of np.nan if the dtype of the underlying array is an integer. So if we want to display nans for the masked regions in the raw view, then we definitely need to convert the array to float64 at some point. I think it's fine if we do that only during the generation of the raw view, if there are masks and the fill value is np.nan. The less invasive changes above would do that.

Is this ready for merge?

Is this ready for merge?

Pending review after my latest changes it should be!

Hello @bnmajor! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2023-10-11 18:36:29 UTC

I made some minor modifications. I think it looks good other than one thing: the panel buffer regions should also have np.nan in them, to match the other views. Try this state file, for example: https://github.com/HEXRD/examples/blob/master/state_examples/Epix_fast_powder/epix_fast_powder.h5

I made some minor modifications. I think it looks good other than one thing: the panel buffer regions should also have np.nan in them, to match the other views. Try this state file, for example: https://github.com/HEXRD/examples/blob/master/state_examples/Epix_fast_powder/epix_fast_powder.h5

Should this change be a part of this PR or part of another one where we've re-thought how to better mesh panel buffers with the masking tools?

In the polar view, the panel buffer values are already nans. We might want to go ahead and do it here too for consistency. Or it can be in another PR. What do you think, @saransh13?

Let’s go ahead and do it in this PR.

@psavery pushed the additional changes we need, the branch looks good in my testing ✅